Skip to content

fix(harness): fail closed at review and privacy boundaries - #19

Merged
phodal merged 2 commits into
QoderAI:mainfrom
CoderMageFox:fix/review-boundary-hardening
Jul 29, 2026
Merged

fix(harness): fail closed at review and privacy boundaries#19
phodal merged 2 commits into
QoderAI:mainfrom
CoderMageFox:fix/review-boundary-hardening

Conversation

@CoderMageFox

@CoderMageFox CoderMageFox commented Jul 29, 2026 •
edited by phodal
Loading

Copy link
Copy Markdown
Contributor

Summary

  • scan secret-bearing Write, Edit, and apply_patch payloads across supported snake_case, camelCase, args, nested, and freeform event shapes
  • scope Qoder home-session evidence per record, exclude foreign or unlinked cwd-less records, and retain home-only data only in an explicit user-global pass
  • make report --no-sessions bypass session probing and reject empty, missing, or non-directory cwd values before collection
  • propagate Git/review collection failures instead of treating them as clean evidence
  • preserve severe finding semantics and prevent cloc from reading symlinks, non-regular files, outside paths, or files beyond the configured size boundary

The implementation and acceptance evidence are recorded in
docs/specs/2026-07-29-11-18-review-boundary-hardening.md.

Issue mapping

Issue Resolution
#11 Normalize tool events and scan write/freeform patch content without echoing secrets
#12 Enforce Qoder workspace ownership at discovery and per-event hydration; keep explicit global evidence labeled user-global
#13 Route --no-sessions directly to the static fallback
#14 Validate and resolve the Git base ref before diff collection; fail closed on errors
#15 Return non-zero sanitized envelopes for argument, Git-probe, and runtime failures
#16 Validate explicit and resolved report --cwd values before starting collectors
#17 Map known severe labels to High and reject arbitrary unsupported labels
#18 Enforce logical, realpath, file-type, descriptor, and bounded-read constraints in cloc

Review

  • Independent code review initially found two High issues; both were fixed and regression tested.
  • Maintainer follow-up found five boundary gaps: mixed/cwd-less Qoder records, missing explicit global analysis, failed Git worktree probes, empty --cwd=, and unbounded cloc reads. All are fixed with regressions in db3963f.
  • Independent architecture review confirmed the fail-closed chain after the fixes. Final status: non-blocking WATCH.
  • Residual watch: platforms without O_NOFOLLOW retain a narrow hostile concurrent swap-to-symlink race. Stable symlinks and outside paths are rejected before reading.

Validation

  • npm run check — full Node suite and package verification passed
  • review-follow-up focused run — 80 tests passed
  • node --test test/doc-link-graph.test.mjs — 6 tests passed
  • node --check for every modified implementation module
  • git diff --cached --check and post-update git diff --check

Closes #11
Closes #12
Closes #13
Closes #14
Closes #15
Closes #16
Closes #17
Closes #18

@CoderMageFox OmX
fix(harness): fail closed at review and privacy boundaries
878287a 
Honor workspace, filesystem, and review boundaries before collecting evidence, and propagate invalid paths, Git refs, and collector failures as explicit errors. Normalize write-tool payloads and preserve severe findings without silently lowering risk.

Implements QoderAI#11, QoderAI#12, QoderAI#13, QoderAI#14, QoderAI#15, QoderAI#16, QoderAI#17, and QoderAI#18 using docs/specs/2026-07-29-11-18-review-boundary-hardening.md. The change splits cleanly across Secret Guard, Qoder session ownership, quickstart input validation, Git/review failure propagation, severity repair, and cloc containment. Validated with npm run check (852 tests plus package verification), the 6-test documentation link graph, focused boundary tests, syntax checks, and staged diff checks.

Constraint: Preserve successful-path and non-blocking finding behavior across supported hosts
Rejected: Treat boundary failures as empty or warning-only evidence | this hides review and privacy failures
Confidence: high
Scope-risk: moderate
Reversibility: clean
Directive: Keep workspace, Git, and filesystem boundary failures explicit when adding collectors
Tested: npm run check; node --test test/doc-link-graph.test.mjs; focused 68-test boundary run; node --check; git diff --cached --check
Not-tested: Hostile concurrent symlink replacement on platforms without O_NOFOLLOW
Co-authored-by: OmX <omx@oh-my-codex.dev>
@phodal
phodal requested a review from Copilot July 29, 2026 04:28
Copilot AI reviewed Jul 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 聚焦于 Better Harness 的“隐私边界 + 审查边界”加固：当无法验证 review / git / cwd / filesystem 边界时改为 fail closed，并补齐 Secret Guard 对多种工具事件形状与写入内容的扫描覆盖，同时保持成功路径的既有语义与降级策略。

Changes:

  • Secret Guard：统一 snake_case / camelCase / args / nested / freeform 的 PreToolUse 事件形状，并扫描 Write/Edit/apply_patch 的内容字段（且不回显 secret）。
  • Qoder 会话分析：workspace 模式下默认排除无法证明归属的 home-only sessions；report --no-sessions 跳过 session probe 并走静态 software-fluency 路由；report --cwd 在 collector 前进行目录有效性校验。
  • Blast-radius / Findings / CLOC：验证 git base ref、在 git 失败时返回明确 fail-closed 报告；severity 修复对已知“重度”标签做保守映射并拒绝任意未知值；cloc 读取加入 realpath/类型/边界与（可用时）O_NOFOLLOW 防护并跳过非 regular 文件。

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/session-usage-summary.test.mjs 增加 usage-summary CLI 覆盖：验证 workspace 结果不包含无关 home-only session。
test/session-analysis.test.mjs 覆盖 Qoder analyzer 在 sessions/show/facets/insights/file-reads 等路径上排除无关 home-only session。
test/review-trigger.test.mjs 覆盖 review-trigger CLI 参数/运行时失败必须非零退出且不泄露输入；覆盖 base ref 不可用时 fail closed。
test/harness-quickstart.test.mjs 覆盖 --no-sessions 静态路由与 --cwd 非法值的提前拒绝；验证有效目录仍保留部分降级。
test/harness-findings-repair.test.mjs 覆盖 severity：对 Critical/Blocker 做保守映射（High），未知值不再被静默降级并应触发校验失败。
test/cloc.test.mjs 覆盖 tracked symlink、outside-repo 路径候选与 dotdot 段名边界行为，确保不泄露外部目标。
test/blast-radius.test.mjs 覆盖 base ref 有效且 clean 的保持 clean；base ref 不可用时 fail closed 并带清晰原因。
test/agent-guardrails-secret-scan.test.mjs 覆盖 Write/Edit/apply_patch 内容扫描、freeform apply_patch、camelCase 与 args/nested 规范化，且不回显 secret。
scripts/session-analysis/platforms/qoder.mjs home-session 探针加入 workspace 归属验证与 planningScope 标注，避免将无关 home-only 归入 workspace。
scripts/review-trigger/cli.mjs CLI 参数错误/运行时错误输出标准化失败 envelope（JSON/非 JSON），并对 cwd 做目录断言；不再吞掉关键边界失败。
scripts/harness-quickstart/cli.mjs --no-sessions 静态 session route；--cwd 在收集前校验并返回结构化错误。
scripts/harness-analysis/repair-findings-json.mjs severity 映射扩展为保守的“重度”标签集合；未知 severity 不再被自动降级到 Medium。
scripts/cloc/count-file.mjs 增加路径 containment（logical+realpath）、file-type 检查与（可用时）O_NOFOLLOW 打开策略，跳过非 regular/越界目标。
scripts/agent-guardrails/secret-scan.mjs PreToolUse 事件规范化并新增写入内容扫描目标集合（Write/Edit/apply_patch），保持 redact。
hooks/git-scripts/blast-radius/git.mjs 引入 GitFailureError 与 baseRef 校验；diff/numstat/untracked 收集失败不再被当作“空 diff”。
hooks/git-scripts/blast-radius/analysis.mjs 将 GitFailureError 转换为明确的 fail-closed 报告（critical + shouldReview）。
docs/specs/2026-07-29-11-18-review-boundary-hardening.md 新增与 #11-#18 对齐的规格文档（AC/非目标/计划/测试证据映射）。

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@phodal
fix(harness): close review boundary gaps
db3963f 
Filter Qoder home-session evidence per record while preserving explicit user-global analysis. Fail review-trigger on Git probe errors, reject empty report cwd values, and bound cloc reads before loading file content.

Completes the PR review follow-ups for QoderAI#12, QoderAI#15, QoderAI#16, and QoderAI#18 under docs/specs/2026-07-29-11-18-review-boundary-hardening.md. Validated with the 80-test focused boundary run, npm run check, the documentation link graph, syntax checks, and staged diff checks.
@phodal
phodal merged commit 305ee58 into QoderAI:main Jul 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment